Skip to content

Conversation

@vkresch
Copy link
Contributor

@vkresch vkresch commented Jan 17, 2020

Reference to a related issue in the repository

The following PR corrected the documentation of units according to the DIN norm in OSI. To complete the correction a test needs to be added which would have failed when the correction was not made.

Add a description

This PR adds a test for checking:

  • Units defintion must be on a separate line (I made a correction in the osi_sensorviewconfiguration.proto file.)
  • The syntax must be: '// Unit: m'
  • First letter of Unit must always be uppercase
  • Units must not be enclosed with any brackets

Mention a member

@jdsika @PhRosenberger pls review and let me know your thoughts and improvements.

Check the checklist

  • My code and comments follow the style guidelines and contributors guidelines of this project.
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests / travis ci pass locally with my changes.

@jdsika jdsika requested a review from PhRosenberger January 17, 2020 10:08
@jdsika jdsika added this to the v3.1.3 milestone Jan 17, 2020
@jdsika jdsika added Documentation Everything which impacts the quality of the documentation and guidelines. Quality Quality improvements. labels Jan 17, 2020
Copy link
Contributor

@jdsika jdsika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. The test already found an inconsitency!

@jdsika jdsika merged commit eee2663 into master Jan 17, 2020
@pmai pmai deleted the unit-test branch April 27, 2020 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Everything which impacts the quality of the documentation and guidelines. Quality Quality improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants